Repository navigation
fix(ui-web): say there is no new key instead of sending an empty save - #884
Conversation
On a connected provider, Update (or Enter) over a key field holding no
new key -- empty, or the stored key the eye revealed, unedited -- sent
model.save_key with the slug alone. A key-shaped provider draws no
address there, so the server could only refuse it, and the page showed
its English refusal ("OpenRouter requires an API key") while the key
was on screen. The page now sends nothing and says, in the pane's own
error line, that there is no new key to save and how to replace it. A
provider whose address is drawn there still saves it.
Acceptance T3.2's observation for that press is updated to match.
Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the five-file change against the current main base. The new guard prevents both empty and unedited-revealed key submissions while preserving address-only saves, and the focused tests cover Update, Enter, the empty field, edited keys, and the existing address-only path.
Covered the diff and surrounding callers, the backend save-key contract, the history that introduced revealed keys, backward compatibility across provider shapes, test-strength changes, AGENTS.md/CLAUDE.md and ui-web/CONTEXT.md rules, localization generation, and the relevant UI ownership boundaries.
Local verification:
- ProviderDetail.test.tsx: 43 passed
- ui-web gen:check: clean
- ui-web type-check: clean
- ui-web ESLint --quiet: clean
- source-language gate for github/main...HEAD: clean
- git diff --check: clean
The broader GitHub page, TUI, RPC-contract, lint, repository, and most unit checks were also green when reviewed.
|
Not a blocker -- from the acceptance pass on head c6e4db7 (merged onto tip be7537c). Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. The new local refusal (ui-web/src/features/settings/providers/ProviderDetail.tsx:125) is gated on Checked on the merged tree: |
Custom, Azure OpenAI and MiniMax CN are endpoint-shaped: the page groups them with local servers, so the no-new-key check skipped them, yet the server refuses every save of theirs that carries no key, the address included. Update over an empty or unedited key field still sent one and showed the English refusal. They now get the same treatment as a key-shaped provider: not yet connected, "Enter the API key first"; connected with nothing changed, "no new key to save"; connected with the address edited, a new line saying the address is saved together with the key, so type the key and press Update. A save that carries a key goes out as before. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
|
Covered in deb3d88: the check now takes endpoint-shaped providers (custom, Azure OpenAI, MiniMax CN) as well, since |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the new endpoint-provider delta and rechecked the full PR against current main. The guard now matches the backend credential contract for custom, Azure OpenAI, and MiniMax CN without changing address-only local providers: unconnected endpoints ask for a key, unchanged connected endpoints send nothing, edited addresses explain that the key must accompany the save, and a typed key still sends the address normally.
Covered the diff, provider-shape helpers and backend credential callers, the prior reveal-key history, backward compatibility across key/endpoint/local shapes, test-strength changes, AGENTS.md/CLAUDE.md and ui-web/CONTEXT.md rules, localization generation, and current discussion. No review threads from my account are open.
Verification on this revision:
- ProviderDetail.test.tsx and SetupBodies.test.tsx: 75 passed
- ui-web gen:check: clean
- ui-web type-check: clean
- ui-web ESLint --quiet: clean
- source-language gate for github/main...HEAD: clean
- git diff --check: clean
- GitHub page checks and TUI checks: passed
|
No blockers in the reviewed scope. Scope: I reviewed the whole PR at What I examined: the gates behind Already filed and now fixed: endpoint-shaped providers skipping the guard is 0xKT's observation of 07:38 UTC. At this head my probe of that path, a connected Findings, both nonblocking:
Verification at this head:
Checked and not reported: the guard's address condition, |
LivXue
left a comment
There was a problem hiding this comment.
Approving at deb3d88df90f: the English refusal is gone for the key and endpoint shapes, nothing that used to change the config stops changing it, and both findings are nonblocking. The summary comment has the verification; the inline note asks to send an edited endpoint address through model.set_fields and to narrow the stated rule, and either can follow this PR.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The new nonblocking thread identifies a valid follow-up: an endpoint address edit can use model.set_fields without asking the user to paste the key again. The current branch does not regress that path--the prior model.save_key request was refused too--and it still improves the empty-save behavior, so this can merge as it stands. The thread also correctly narrows the broad comment about every key-shaped provider: Bedrock is an ambient-credential exception, though suppressing its no-op save remains the right behavior.
Focused verification on the unchanged head: ProviderDetail.test.tsx and SetupBodies.test.tsx, 75 passed. I did not resolve or reply in the other reviewer's open thread.
…y field A connected endpoint provider (custom, Azure OpenAI, MiniMax CN) whose address was edited with no new key typed was told to type the key again. That is model.save_key's rule, not the provider's: an address on its own goes through model.set_fields, the route the settings design gives any field but the key, and the one the Advanced address field already uses. The edited address is now saved that way, and the key-with-address line is gone. The no-new-key line now names the API key field rather than saying "here", since it is drawn at the foot of the pane, and uses "API Key" like the strings around it. The comments no longer say every key-less save of a key-shaped provider is refused: Bedrock's is accepted and changes nothing, which is still no reason to send it. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the delta from the previous head and rechecked the affected full-PR paths. The prior nonblocking endpoint-address concern is addressed: a connected endpoint with only an edited address now writes api_base through model.set_fields, including while the revealed key remains unedited; unchanged key/address state still sends nothing, and a genuinely new key continues through model.save_key. The obsolete copy and generated localization entry were removed consistently.
Covered the delta and full diff, store/source refresh behavior, provider-shape and backend contracts, backward compatibility across key/endpoint/local providers, test-strength changes, current discussion, AGENTS.md/CLAUDE.md and ui-web/CONTEXT.md rules, localization generation, and compatibility with the latest main. No review threads from my account are open; the other reviewer's addressed thread is already resolved.
Verification on this revision:
- ProviderDetail.test.tsx and SetupBodies.test.tsx: 76 passed
- ui-web gen:check: clean
- ui-web type-check: clean
- ui-web ESLint --quiet: clean
- source-language gate for github/main...HEAD: clean
- git diff --check: clean
- merge-tree against current github/main: clean
- GitHub page checks and TUI checks: passed
Summary
On a connected provider's sheet (Settings > Model providers), Update -- or
Enter in the key field -- with no new key in the field sent
model.save_keywith the slug alone. That happens with the field empty, and since #881 with
the stored key the eye revealed, unedited, which is deliberately not sent
back. A key-shaped provider draws no address in that card, so the request
carried nothing to save: the server refused it -- or, for Bedrock's ambient
credential, accepted it and changed nothing -- and the page toasted its
English refusal ("OpenRouter requires an API key") while the key sat on
screen.
ProviderDetail.tsx: when the provider is connected, the field holds nonew key and the card has no address to send, the page sends nothing and
says so in the pane's own error line (where "Enter the API key first"
already appears for a provider not yet connected): there is no new key to
save; to replace the saved one, type a new key and press Update. A
provider whose address is drawn in that card (a key-less vendor reached by
address) still saves the address, as before.
too: the page groups them with local servers, but
model.save_keyrefusestheir save without a key, the address included. Not yet connected, "Enter
the API key first"; connected with nothing changed, the same "no new key"
line; connected with only the address edited, the address is saved on its
own through
model.set_fields-- the route the settings design gives anyfield but the key, and the one the Advanced address field already uses --
so no key is asked for again, even with the revealed key unedited in the
field. A save that carries a key goes out as before.
i18n/messages.json: one new string,gui.settings.providers.key_unchanged(en and zh), naming the API key field since the line is drawn at the foot
of the pane; the TUI catalogue is regenerated.
docs/specs/2026-09-20-web-settings-model-section-acceptance.md: T3.2'sobservation for pressing Update over the revealed key now reads "sends no
frame and the pane says there is no new key to save", and its red-when
column fails any frame from that press.
No layout change: the message uses the existing error line.
Type
Verification
ui-web:gen:checkclean,type-checkclean,eslint .0 errors (thesame 4 warnings as main, in other files),
npx vitest run-- 214 files,3211 tests passed; build and both boot snapshots match their goldens. New
cases in
ProviderDetail.test.tsx: Update and Enter over an uneditedrevealed key send nothing and show the message, and so does Update over
the empty field; for a custom row, nothing changed shows the same message,
an edited address is saved alone through
model.set_fields(also with therevealed key unedited in the field), an edited address with a key typed
saves both, and Connect on one not yet connected asks for the key first;
the existing case that saves a key-less vendor's address still passes.
ui-tui:lint:i18nandtype-checkclean;npm test-- 144 files,2111 tests passed
Mutations, each turning a test red: dropping the new check (the empty save
is sent again), dropping its address condition (a key-less vendor's
address is no longer saved), leaving endpoint providers out, not asking an
unconnected endpoint provider for the key first, refusing an edited
endpoint address again, and writing an unchanged one anyway
Server side, for the claims above:
credential_statusrefuses custom,azure_openai and minimax_cn_api without a key whether or not an address is
sent, and accepts each with one;
model.set_fieldswithapi_basealoneis how the Advanced address field already saves
Real browser, on an isolated gateway running this branch (its own home
holding only a made-up key, its own ports, channels off): Settings > Model
providers > the connected provider; eye pressed, then Update -- the page
sent only the
model.reveal_keycall, nomodel.save_key, and the paneshowed the new message; eye pressed again, then Update over the empty
field -- no frame, the same message; the config file's hash was unchanged;
no console errors
scripts/check_source_language.py,scripts/check_commit_messages.pyandscripts/check_large_files.pyon the merge base, and the prospectivesquash header through
commitlint-- all cleanRelevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
No user-facing doc describes the provider key field.
Risk
User-visible change: on a key-shaped or endpoint-shaped provider, Update or
Connect with nothing new shows a sentence in the reader's language instead of
the server's English refusal, and no request is made; a connected endpoint's
edited address now saves without the key. Nothing that used to save stops
saving:
model.save_keyrefused every one of these saves (checked with theprovider's environment variable set as well: refused, stored key unchanged),
except Bedrock's, which it accepts and which leaves the config unchanged. A
provider whose address is drawn in the key card saves exactly as before.
Rollback is a plain revert.
Related Issues
N/A